Reset TLS on Profiling Entrance #999
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: D63924780 broke some tests because of pthread_atfork having strange properties with subsequent calls. To fix this, lets deviate from this method and just reset the TLS whenever we enter the a profiling context. This will ensure that we will start "fresh" for all the PID/TID related content upon every profile. The drawbacks are: 1. 1 Extra Cache Miss per Profile - This is negligible because the cache miss is during the prepare stage for auto-trace and schedule for on-demand. To add to this, the cache miss penalty is very small 2. No Reset if Fork during Profile - If someone were to fork in the middle of a profile the TLS won't get reset. However, there are many other issues that could happen due to a fork midway through a profile such as undefined behavior with cupti, distorted profiling window etc. We shouldn't worry about this case as of today. Differential Revision: D64120658
|
This pull request was exported from Phabricator. Differential Revision: D64120658 |
sraikund16
commented
Oct 9, 2024
| @@ -16,6 +16,7 @@ | |||
| #include "ConfigLoader.h" | |||
| #include "DaemonConfigLoader.h" | |||
| #include "DeviceUtil.h" | |||
| #include "ThreadUtil.h" | |||
aaronenyeshi
approved these changes
Oct 10, 2024
|
This pull request has been merged in 00a00e0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
D63924780 broke some tests because of pthread_atfork having strange properties with subsequent calls. To fix this, lets deviate from this method and just reset the TLS whenever we enter the a profiling context. This will ensure that we will start "fresh" for all the PID/TID related content upon every profile. The drawbacks are:
1 Extra Cache Miss per Profile - This is negligible because the cache miss is during the prepare stage for auto-trace and schedule for on-demand. To add to this, the cache miss penalty is very small
No Reset if Fork during Profile - If someone were to fork in the middle of a profile the TLS won't get reset. However, there are many other issues that could happen due to a fork midway through a profile such as undefined behavior with cupti, distorted profiling window etc. We shouldn't worry about this case as of today.
Differential Revision: D64120658